Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

moved main() into __main__.py to keep __init__.py minimal #67

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

seancmonahan
Copy link

print("Call your main application code here")


if __name__ == "__main__":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__name__ will never not equal __main__ in __main__.py.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__name__ equals sample.__main__ once the project is installed and run via the executable 'sample' generated by setup.py.

Without the if __name__ == "__main__": check (but still calling main() at the end of __main__.py):

$ virtualenv foo
$ source foo/bin/activate
(foo) $ pip install ../path/to/sampleproject
(foo) $ sample #i.e. foo/bin/sample
Call your main application code here
Call your main application code here
(foo) $

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose the same effect could be had by reverting __init__.py and setup.py, and making __main__.py be (like pip's __main__.py):

from . import main

if __name__ == '__main__':
    main()

My motivation was twofold though:

  • Make the package runnable via python -m sample
  • Keep __init__.py as clean as possible.

For example, pip's primary __init__.py is a single line definition of __version__, and setup.py uses pip._internal:main as its console_script entrypoint. (I do see that ultimately main() is defined inside pip/_internal/__init__.py ¯\_(ツ)_/¯ )

Regardless of where def main(): ends up, my first goal (to make the package runnable) requires the addition of a __main__.py.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right about __name__ during import...

IMO, the double execution is evidence of abuse. __main__.py is meant to control the behavior of python -m, yet here it is tangled up in the execution of the entry point.
Instead of __main__.py and the entry point leading the user to the same call (e.g. sample.cli.main()), the latter leads to the former which has to have a special hack added to make things work.

The docs even describe __main__.py as the equivalent of if __name__ == '__main__' for packages (implying packages should not need it):

For a package, the same effect can be achieved by including a __main__.py module, the contents of which will be executed when the module is run with -m.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, you're correct about the double-execution behavior. Seems that I was accidentally running from a virtualenv with an outdated install of sample with the console_script entrypoint pointing to __main__ still! Should I close this pull request?

Would a single patch for adding a __main__.py consisting solely of

from . import main

main()

be accepted/useful? It would allow the package to be run via python -m sample and avoid the nonsense I would've introduced by moving def main(): into __main__.py.

Also, would adding a sample/cli.py module be in the scope of this project? It seems like a useful way to make minimize __init__.py to either an empty file or a single from .cli import main. (This does trigger a flake8 error for an unused import.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is a reasonable __main__.py, and I would say a cli.py module is in-scope for the project since someone previously defined an entry_point.

Not sure how we want to manage imports in __init__.py... I think generally people use that file to define the package API (which often implies unused imports), but it's not explicitly required since modules within the package can be imported independently. I'd probably just leave it empty to avoid a # noqa.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this is not something that has an "obvious" best practice approach (as evidenced by the discussion going on here) and users would be better served with documentation that discussed the various approaches and the pros and cons, rather than having one particular approach added to this sample project. Let's also not forget that most projects don't even need to be runnable.

A new section in https://packaging.python.org/guides/ would likely be a better way of dealing with this, IMO.

Base automatically changed from master to main January 21, 2021 18:40
@DiddiLeija
Copy link
Member

Hi @seancmonahan. Are you still interested on this PR? If you are, I recommend you to edit the first post of the PR, and add the following expression: Closes #141. That way, that issue (#141, which is directly related) will be closed if this gets merged. Also, can you rebase the PR?

@DiddiLeija
Copy link
Member

Hi everyone! Are we still interested in this PR?

@pfmoore
Copy link
Member

pfmoore commented Dec 30, 2022

My objection to doing this, which I noted above, still stands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants